Allow modification of existing attributes and text content. #23
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ignoring my previous issue, I ended up stumbling across an issue when modifying the value of attributes that were already attached to an element, as well as modifying existing text content.
The previous behavior for
setAttribute
was to simply append a new attribute to theattrs
array, while this does work for new elements, it causes issues when attempting to override existing attributes. When parse5 serializes the AST, it only recognizes the first attribute of a type, and ignores any others with the same name. This behavior is un-intuitive as callingsetAttribute
in some situations may have no effect on the serialized content.This issue is easily fixed by simply removing any old attributes with the given name.
I also stumbled across a very similar issue with
setTextContent
when modifying the text content of some elements. This issue was also caused by appending new text nodes without removing any previous content beforehand. While the old behavior does act similar to thedocument.write()
function, the namesetTextContent
doesn't match the functionality. If this behavior is intended, I would recommend renamingsetTextContent
toappendTextContent
and creating a new function forsetTextContent
which behaves intuitively.This issue was also easily fixed by removing any text nodes from a parent element before appending the new content.